Allow the quickstart compose file's envvars to be overridden #112
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
September 20, 2018 11:49
The quickstart `docker-compose.yml` file hardcodes some parameters. Docker Compose supports an interpolation mechanism which allows the same defaulting to happen, but in a way which permits the user to modify these parameters when running docker-compose. Making this change will allow this Compose file to be used by more consumers, whilst still enabling the same low-friction quickstart workflow. Alternative mechanisms, such as adding a `docker-compose.env` file and wget'ing it as part of the quickstart, are possible - and don't increase the quickstart friction very much. This commit implements the mechanism which doesn't change the quickstart experience *at all*.
This commit changes the prehashed password for the `test` user in the quickstart `docker-compose.yml` file to the same password, but in plaintext. This makes it clearer for consumers of this file as to how they should change the default user and password. Suggesting in the status quo that a hash must be specified, especially one which will contain dollar signs, makes it less likely a user will override this setting using the functionality introduced in `7f2e0a2`.
jpluscplusm
pushed a commit
to alphagov/paas-bootstrap
that referenced
this pull request
Sep 24, 2018
Concourse upstream publishes a docker-compose file which uses their new `quickstart` command to auto-wire a worker and atc together. This commit uses that upstream file (vendorered, as a couple of changes need to be accepted upstream before we can use it [1]). To use it, we needed to upgrade to Bionic as our bootstrap substrate for a later version of docker-compose, which introduced a subtle cloud-init boot-time timing problem, where Vagrant can run our commands (`apt-get update`) before the apt config files are pointing at the correct repos. [1] concourse/docs#112
jpluscplusm
pushed a commit
to alphagov/paas-bootstrap
that referenced
this pull request
Sep 24, 2018
Concourse upstream publishes a docker-compose file which uses their new `quickstart` command to auto-wire a worker and atc together. This commit uses that upstream file (vendorered, as a couple of changes need to be accepted upstream before we can use it [1]). To use it, we needed to upgrade to Bionic as our bootstrap substrate for a later version of docker-compose, which introduced a subtle cloud-init boot-time timing problem, where Vagrant can run our commands (`apt-get update`) before the apt config files are pointing at the correct repos. [1] concourse/docs#112
jpluscplusm
pushed a commit
to alphagov/paas-bootstrap
that referenced
this pull request
Sep 24, 2018
Concourse upstream publishes a docker-compose file which uses their new `quickstart` command to auto-wire a worker and atc together. This commit uses that upstream file (vendorered, as a couple of changes need to be accepted upstream before we can use it [1]). To use it, we needed to upgrade to Bionic as our bootstrap substrate for a later version of docker-compose, which introduced a subtle cloud-init boot-time timing problem, where Vagrant can run our commands (`apt-get update`) before the apt config files are pointing at the correct repos. [1] concourse/docs#112
jpluscplusm
pushed a commit
to alphagov/paas-bootstrap
that referenced
this pull request
Sep 24, 2018
Concourse upstream publishes a docker-compose file which uses their new `quickstart` command to auto-wire a worker and atc together. This commit uses that upstream file (vendorered, as a couple of changes need to be accepted upstream before we can use it [1]). To use it, we needed to upgrade to Bionic as our bootstrap substrate for a later version of docker-compose, which introduced a subtle cloud-init boot-time timing problem, where Vagrant can run our commands (`apt-get update`) before the apt config files are pointing at the correct repos. [1] concourse/docs#112
|
Hmm I'm a bit lukewarm on this. Thanks for submitting it, obviously, and sorry for the delay - we're in flux right now with concourse/concourse#2534 and that's slowing down our PR influx. The added flexibility is nice but I'm not sure we want the quick start being used for much more than the initial tire-kicking, and I'm afraid that by opening it up to more configuration we'll make it look more like a real-world Concourse deployment template. We don't intend to maintain this file for use by downstream deployments - it's just there as an example. More real-world use cases should probably use a full-fledged Docker Compose file (and forego Thanks again and sorry for the wait! |
|
Closing for now based off @vito 's feedback. Thanks for making hte PR though! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The quickstart
docker-compose.ymlfile hardcodes some parameters. Docker Compose supports an interpolation mechanism which allows the same defaulting to happen, but in a way which permits the user to modify these parameters when running docker-compose.Making this change will allow this Compose file to be used by more consumers, whilst still enabling the same low-friction quickstart workflow.
Alternative mechanisms, such as adding a
docker-compose.envfile and wget'ing it as part of the quickstart, are possible - and don't increase the quickstart friction very much. This commit implements the mechanism which doesn't change the quickstart experience at all.This PR also makes it clearer to the prospective overriding user how to set the user password they want.
Tests
I'm not including any tests. Here's my rationale:
if there already exist any CI/etc tests of the
docker-compose.ymlfile, then this PR will either break them, or it won't. If it doesn't, and the status quo experience (i.e. no externally set envvars) doesn't change, then by definition the status quo has been maintained. Even if the additional functionality intended to be implemented here doesn't work, nothing of value was lost.if there aren't any tests of this file, then implementing a test framework to run up machines on which to run this set of containers seems like something that needs project-level organisation, and isn't in scope for a drive-by PR'ing. IMHO :-)